-
-
Notifications
You must be signed in to change notification settings - Fork 137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for browser profiles #113
Conversation
Hey, Thanks for for working on this! There is another pull request adding support for Google Chrome profiles which has a lot of similarity to your approach, though it doesn't attempt to add profile support for Brave or Firefox at this point. I haven't been able to test your branch yet, but unless I am mistaken you would have to refer Google Chrome profiles by the folder name instead of the actual profile name here (see this comment. The JavaScript file The pull requests are both pretty similar and I think we can find a good solution that uses parts of both. There haven't been any updates in the last couple of weeks to the other one so I'm not sure it's still being worked on. I can probably have a look this weekend at combining these efforts. |
Yes, I've been keeping an eye on that for a while hoping it would get more traction, in the end got impatient and started experimenting for a fix myself. I think my solution provides a more generalized approach. It is inspired by the original PR. I should have probably linked to it or used it as my fork.
I think with all Chromium browsers that haven't messed with the default CLI options (cough Firefox) If the I personally being slightly annoyed by that, first created the directory with my intended names and then renamed my profiles. Keeping things consistent. With Firefox, you can either provide the profile name with I chose call the config property in Finicky
Thanks, for some reason the folder was hidden by XCode and AppCode IDEs, I failed to realize that's where those files were coming from. Naively just updated the JS and experimented locally. I'll update the TS scripts. Do I just need to run
Thanks for creating this project, has been really useful for me and my personal workflows. Look forward to any suggestions you have to improving this PR. |
You need yarn to build the js counterpart. After installing all
dependencies by running `yarn` you should just be able to run `yarn build`
to update the bundled js.
I will happily help out any way I can to test this or to fix any issues. It
would be an amazing addition to finicky.
I'm working towards a bigger update pretty soon and if we can get this in
it would be great.
Thanks!
…On Fri, May 15, 2020, 20:03 Vivek Bheda ***@***.***> wrote:
Hey,
Thanks for for working on this! There is another pull request
<#95> adding support for Google
Chrome profiles which has a lot of similarity to your approach, though it
doesn't attempt to add profile support for Brave or Firefox at this point.
Yes, I've been keeping an eye on that for a while hoping it would get more
traction, in the end got impatient and started experimenting for a fix
myself. I think my solution provides a more generalized approach. It is
inspired by the original PR <#95>.
I should have probably linked to it or used it as my fork.
I haven't been able to test your branch yet, but unless I am mistaken you
would have to refer Google Chrome profiles by the folder name instead of
the actual profile name here (see this comment
<#95 (comment)>.
I think with all Chromium browsers that haven't messed with the default
CLI options (*cough* Firefox) --profile-directory=<profile name> points
to the profile directory. However when a user creates a new profile, the
directory and the profile have the same name. The directory isn't renamed
when the user renames the profile.
If the --profile-directory=<profile name> directory doesn't exist, the
browser will just create a directory with that name and generate a profile.
However names the profile with the default name 🤦
I personally being slightly annoyed by that, first created the directory
with my intended names and then renamed my profiles. Keeping things
consistent.
With Firefox, you can either provide the profile name with -P or provide
the profile directory path with -profile.
I chose call the config property in Finicky profile since you were
generally referring to the profile name (or at least what they were
originally called)
The JavaScript file Finicky/Finicky/finickyConfigAPI.js is generated from
the TypeScript config-api project in the repo root so any changes made
need to be added there. The profile field would need to be added to the
TypeScript types as well as the schema validation.
Thanks, for some reason the folder was hidden by XCode and AppCode IDEs, I
failed to realize that's where those files were coming from. Naively just
updated the JS and experimented locally. I'll update the TS scripts.
Do I just need to run npm run build to regenerate the JS files from the
TS files?
The pull requests are both pretty similar and I think we can find a good
solution that uses parts of both. There haven't been any updates in the
last couple of weeks to the other one so I'm not sure it's still being
worked on.
I can probably have a look this weekend at combining these efforts.
Thanks for creating this project, has been really useful for me and my
personal workflows. Look forward to any suggestions you have to improving
this PR.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#113 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGYKIZLFEDWC5SWMDW7JVTRRV7WHANCNFSM4NA6S2BQ>
.
|
Hi, sorry I've not been able to devote time to this PR for the last few weeks. I'll be updating it this week. |
8749d0f
to
c0135c4
Compare
Hi @johnste I've added changes as you requested to the I am wondering if you require any additional changes/unit tests for this to be mergeable. Let me know, I should have more bandwidth available this week to address any required changes. Thanks, |
c0135c4
to
e42adc4
Compare
Figured my way around |
Thanks, I'll check this out. Did you have to disable Hardened runtime to be able to test? I'm sorry for making it harder but I needed to enable it to be able to noterize the app. |
I don't think it was the hardened runtime itself, I was just having a hard time building because of the code signing. Since I wasn't using Xcode it was difficult for me to understand how to turn it off. I eventually gave in and opened Xcode and found the option to build for local testing. (I don't have experience working with Apple app development) Now that code signing is part of the default build configuration probably should update the command in the README/Wiki to build locally?
|
Yes, thank you. I will update the wiki.
Finicky is also my one and only XCode project so I'm learning as I go.
|
Hey I had some time to test things out a bit. I didn't have much success so far. I was only able to test Firefox tonight. I added a handler like so: {
match: /example3/,
browser: {
name: "Firefox",
profile: "simple",
},
}, I ran I added a debug output to show the command Finicky generated which is I tried commenting out I will try to dig more when I have the time again. I could test the Chromium based and see if that works better for me. |
I've been using a build using this branch for my Brave browser profiles. I think Chromium based browsers will function fine. With respect to the open command behaving differently, I am assuming its because of its running in a subshell. If we use native AppKit Classes that control the |
Hi, sorry for the lack of updates on this. I found the issue with my changes. The issue was two parted:
to
I tested this locally and should support firefox profiles correctly now. |
I guess, just needed some fresh eyes. |
Let me know once this is mergeable, I can go ahead and squash my commits. |
Thank you so much for working on this @bhedavivek 🥇 |
This is great! Looking forward to see this merged, it'll be super useful. |
Hey, I haven't been able to get the time necessary to test this out yet, will do when my schedule opens up. If anyone else wants to help out, building it and trying it out would help a ton! |
I tried this PR with the following settings and found that it works well. module.exports = {
defaultBrowser: "Google Chrome",
handlers: [
{
match: (a) => {
finicky.log(JSON.stringify(a, null, 2));
},
browser: "Google Chrome",
},
{
match: ({ keys }) => {
return keys.command;
},
browser: {
name: "Google Chrome",
appType: "appName",
profile: "Default"
}
},
{
match: ({ keys }) => {
return keys.shift;
},
browser: {
name: "Google Chrome",
appType: "appName",
profile: "Profile 1"
}
}
]
}; @bhedavivek Thank you for a great job! |
Could we please have this merged? 🙏 |
Will try to get to this soon. I haven't been able to test this yet and I don't want to merge without some thorough testing. |
Been using this as my local build for the past 3 months with Brave Browser, incase that helps instill some confidence with support for Chromium based browser profiles. |
I have my development setup up and running again, should be able to test this during the next few days. |
After testing a bit this seems to work well with Chrome (and, like you said, assuming other Chromium based browsers). I noted a couple of things:
I didn't have much success getting Firefox profiles to work, it appears to work intermittently. Overall, I'm happy to merge this and start preparing a new release. I think until we have more ability to test Firefox profiles (or other browsers), it makes sense to only officially support Chromium profiles for now. If you want to look into any other issues or prepare this pull request for merging, feel free to do that and I will merge when you're done. Thank you so much for you work @bhedavivek and sorry for me taking such a long time to get back to you. |
Fixing the chromium profile names with spaces should be easy, I'll escape the profile name for the chromium browsers. For disabling firefox support, would you like me to comment out the code or remove it completely? |
Commenting out is fine, maybe add a short explanation as to why/link to
comment maybe
Thanks!
…On Tue, Aug 11, 2020, 21:59 Vivek Bheda ***@***.***> wrote:
After testing a bit this seems to work well with Chrome (and, like you
said, assuming other Chromium based browsers).
I noted a couple of things:
- If the profile doesn't exist, Chrome will create a new profile
- If the profile name uses a space character, Chrome doesn't appear to
be able to match it, and instead creates a new profile
I didn't have much success getting Firefox profiles to work, it appears to
work intermittently.
Overall, I'm happy to merge this and start preparing a new release. I
think until we have more ability to test Firefox profiles (or other
browsers), it makes sense to only officially support Chromium profiles for
now.
If you want to look into any other issues or prepare this pull request for
merging, feel free to do that and I will merge when you're done. Thank you
so much for you work @bhedavivek <https://github.com/bhedavivek> and
sorry for me taking such a long time to get back to you.
Fixing the chromium profile names with spaces should be easy, I'll escape
the profile name for the chromium browsers.
For disabling firefox support, would you like me to comment out the code
or remove it completely?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#113 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGYKI7QGLZ4HFJJRM6H5CTSAGPLLANCNFSM4NA6S2BQ>
.
|
Allows selecting profile of target browser. Adds support for Google Chrome, Brave Browser and Mozzila Firefox.
88bec78
to
d0b92a0
Compare
Disabling support for Firefox browsers till there is better testing methodology and tooling for Firefox browsers. Performs unreliably when targeting specific profiles for Firefox. Github PR Comment: johnste#113 (comment)
d0b92a0
to
9756d09
Compare
I updated the method to only modify the browser command for supported browsers in 9054810 |
Disabling support for Firefox browsers till there is better testing methodology and tooling for Firefox browsers. Performs unreliably when targeting specific profiles for Firefox. Github PR Comment: johnste#113 (comment)
Updates to `getProfileOption` method now only returns a valid profile option string array for supported browsers, returns nil otherwise. This will keep the browser command practically the same when trying to launch with unsupported browsers.
9756d09
to
9054810
Compare
Disabling support for Firefox browsers till there is better testing methodology and tooling for Firefox browsers. Performs unreliably when targeting specific profiles for Firefox. Github PR Comment: #113 (comment)
Thank you @bhedavivek for you great work! |
@johnste 👋 Will you consider pushing out a new release with this anytime soon? 🙏 |
Yes, I'll try to get a release out later tonight. Edit: More likely tomorrow. Writing release notes takes a bit of time. |
Released as 3.1 https://github.com/johnste/finicky/releases/tag/v3.1.0 |
Allows selecting profile of target browser.
Adds support for Google Chrome, Brave Browser and Mozzila Firefox.
Apologies in advance for not following any Swift/Javascript conventions correctly. I don't don't have much experience in either.
You can specify which browser profile target by defining the profile property on the browser object in the config file.
Sample Config